Harden HTTP bridge previous_response_id continuity recovery#301
Harden HTTP bridge previous_response_id continuity recovery#301JoshuaRileyDev wants to merge 1 commit intoSoju06:mainfrom
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1960fa902
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if response_id is not None and matched_request_state is not None: | ||
| await self._register_http_bridge_previous_response_id(session, response_id) |
There was a problem hiding this comment.
_process_http_bridge_upstream_text only writes to _http_bridge_previous_response_index when matched_request_state is set, but the function explicitly supports a fallback where response.created is missing and _pop_terminal_websocket_request_state(...) resolves the request at terminal time. In that fallback, matched_request_state stays None, so successful responses with a terminal response.id are never indexed, and a follow-up request using that previous_response_id can incorrectly fail with previous_response_not_found even though continuity is still live.
Useful? React with 👍 / 👎.
| if previous_response_id is not None and inflight_future is None and ( | ||
| existing is None or existing.closed or existing.account.status != AccountStatus.ACTIVE |
There was a problem hiding this comment.
This guard blocks previous-response recovery whenever an inflight session future exists for the request-derived key, and the method then sets a continuity error for any request with previous_response_id. That means a valid mapped live session can be ignored during concurrent traffic (for example, another request is currently creating a session for the same prompt-cache key), causing a false 400 continuity failure instead of reusing the known session.
Useful? React with 👍 / 👎.
|
does this solve #221? |
3 participants
Summary
Validation
uv run pytest -q tests/integration/test_http_responses_bridge.py -k "missing_turn_state_alias_with_previous_response_id_fails_closed or recovers_previous_response_without_turn_state_alias or send_failure_returns_previous_response_not_found"